Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Small optimizations #320

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

tudorpintea999
Copy link

Hello Initia team!
I have refactored a small part of your code
Not much but it s honest work and hope it helps you
Br,
Tudor.

Renamed variables for clarity (e.g., ak → authKeeper, num → currentAccountNum).
	•	Added more descriptive comments for each step of the process.

	•	Provided more informative error messages using sdk.ErrInternal and sdk.ErrInvalidRequest.
	
	•	Added a type assertion with a fallback to handle potential incorrect types for AccountKeeper.
	
	•	Highlighted the rationale for using a gas-free context when interacting with the account number.
	
	•	Clearly defined the increment value (1_000_000) and its variation during simulation to make the logic easier to understand.

	•	Retained flexibility to handle additional modes by centralizing the execution mode check.
Broke down large functions (AnteHandle) into smaller, focused helper methods (verifySignature, performSignatureVerification, etc.).
	
	•	Clearer variable names (sig → signature, acc → account).
	•	Simplified nested logic by early returns and helpers.
	•	Added context-specific error messages for better debugging.
	•	Ensured consistency in gas calculation logic.
	•	Centralized repeated operations into helper functions (e.g., performSignatureVerification).
	•	Modularized logic for easier testing and extension.
	validateHandlerOptions: Validates that all required dependencies are provided.
	•	getTxFeeChecker: Retrieves the appropriate fee checker.
	•	buildAnteDecorators: Constructs the AnteDecorator chain.


	•	Grouped related operations together for easier understanding.


	•	Enhanced the readability of the free lane fee checker by clarifying its logic.




	•	Kept the design flexible by accommodating custom fee checkers (TxFeeChecker) and defaulting to a predefined one if not provided.
Added validateMempoolConfig to encapsulate configuration validation.
	•	Centralized validation logic improves readability and reuse.


	•	Replaced nested if-else statements with if checks and early returns.




	•	Encapsulated logic in modular helper functions (getTxKey, AssertLaneLimits) to reduce code duplication.


	•	Provided hooks to customize transaction priority (Priority) and validation logic.


	•	Optimized Contains and Remove to avoid redundant checks by using getTxKey.
Created helper functions like validateTxEligibility, validateTxPriority, and handleNonMatchingTransactions to reduce code duplication and enhance clarity.


	•	Encapsulated repeated logging and transaction removal logic into logAndAppend and logTxLimitExceeded.


	•	Used descriptive variable names (txsToInclude, txsToRemove) for better understanding.
@tudorpintea999 tudorpintea999 requested a review from a team as a code owner December 15, 2024 10:51
Copy link

coderabbitai bot commented Dec 15, 2024

📝 Walkthrough

Walkthrough

This pull request introduces comprehensive improvements across several core modules of the blockchain application, focusing on code readability, error handling, and modular design. The changes span the ante handler, account number management, signature verification, mempool, and proposal handling components. Key modifications include renaming variables for clarity, extracting complex logic into separate methods, enhancing error messages, and refactoring validation processes. The overall goal appears to be improving code maintainability and providing more robust transaction processing mechanisms.

Changes

File Change Summary
app/ante/accnum/account_number.go - Renamed ak to accountKeeper
- Enhanced error handling and comments
- Updated increment logic for account numbers
app/ante/ante.go - Added validation function validateHandlerOptions
- Created getTxFeeChecker and buildAnteDecorators functions
- Improved comments and code organization
app/ante/sigverify/sigverify.go - Introduced performSignatureVerification method
- Added wrapSignatureError helper function
- Refined error handling and signature verification logic
app/lanes/mempool.go - Added validateMempoolConfig function
- Simplified transaction caching logic
- Improved error handling in various methods
app/lanes/proposals.go - Added multiple helper methods for transaction validation
- Extracted complex logic into separate methods
- Enhanced logging and error handling

Sequence Diagram

sequenceDiagram
    participant TX as Transaction
    participant Ante as AnteHandler
    participant AccNum as AccountNumberDecorator
    participant SigVerify as SigVerificationDecorator
    participant Mempool as Mempool

    TX->>Ante: Submit Transaction
    Ante->>AccNum: Validate Account Number
    AccNum-->>Ante: Account Number Validated
    Ante->>SigVerify: Verify Signatures
    SigVerify-->>Ante: Signatures Verified
    Ante->>Mempool: Insert Transaction
    Mempool-->>Ante: Transaction Inserted
Loading

Possibly Related PRs

Suggested Reviewers

  • sh-cha
  • Eric-Warehime
  • Vritra4

Poem

🐰 Hoppity hop through the code's domain,
Refactoring logic with algorithmic gain,
Decorators dance, errors now clear,
Modularity blooms, the path becomes sheer!
A rabbit's delight in clean software's refrain 🌟

Tip

CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command @coderabbitai generate docstrings to have CodeRabbit automatically generate docstrings for your pull request. This feature will be included in our Pro Plan when released.


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🔭 Outside diff range comments (1)
app/lanes/mempool.go (1)

Line range hint 33-70: Consider adding validation for the extractor parameter.

While the validation logic is well-organized and comprehensive for ratio and txEncoder, the extractor parameter should also be validated to ensure it's not nil, as it's a critical component for transaction processing.

 func validateMempoolConfig(ratio math.LegacyDec, txEncoder sdk.TxEncoder) error {
     if !ratio.IsPositive() {
         return errors.New("mempool creation: ratio must be positive")
     }
     if ratio.GT(math.LegacyOneDec()) {
         return errors.New("mempool creation: ratio must be less than or equal to 1")
     }
     if txEncoder == nil {
         return errors.New("mempool creation: tx encoder is nil")
     }
     return nil
 }

 func NewMempool[C comparable](
     txPriority blockbase.TxPriority[C], extractor signer_extraction.Adapter,
     maxTx int, ratio math.LegacyDec, txEncoder sdk.TxEncoder,
 ) (*Mempool[C], error) {
+    if extractor == nil {
+        return nil, errors.New("mempool creation: signer extractor is nil")
+    }
     if err := validateMempoolConfig(ratio, txEncoder); err != nil {
         return nil, err
     }
🧹 Nitpick comments (5)
app/lanes/proposals.go (3)

165-167: Avoid logging nil errors in logAndAppend

In logAndAppend, the error is logged even when err is nil, which could lead to misleading log entries. Check if err is not nil before including it in the log.

Apply this diff to conditionally log the error:

 func (h *DefaultProposalHandler) logAndAppend(message string, tx sdk.Tx, err error, txsToRemove *[]sdk.Tx) {
-    h.lane.Logger().Info(message, "err", err)
+    if err != nil {
+        h.lane.Logger().Info(message, "err", err)
+    } else {
+        h.lane.Logger().Info(message)
+    }
     *txsToRemove = append(*txsToRemove, tx)
 }

142-144: Correct the transaction priority error message

The error message in validateTxPriority might be misleading. When priority == -1, it means the current transaction has lower priority than the previous one. The message should reflect that to avoid confusion.

Update the error message as follows:

 return fmt.Errorf("tx at index %d has lower priority than tx at index %d", index, index-1)

122-124: Ensure transactions exceeding size limits are handled consistently

In validateTxEligibility, when a transaction causes updatedSize to exceed limit.MaxTxBytes, it logs the issue but does not append the transaction to txsToRemove. This could result in the transaction being reconsidered in future proposals. Consider appending the transaction to txsToRemove for consistency.

Apply this diff:

 return false
+ *txsToRemove = append(*txsToRemove, tx)
app/ante/accnum/account_number.go (1)

51-53: Handle potential errors when setting the new account number

In AnteHandle, when setting the new account number using authKeeper.AccountNumber.Set, you wrap the error with sdk.ErrInternal. Since sdk.ErrInternal is deprecated, consider using sdkerrors.ErrInvalidRequest or another appropriate error type from sdkerrors.

Update the error handling as follows:

 if err := authKeeper.AccountNumber.Set(gasFreeCtx, newAccountNum); err != nil {
-    return ctx, sdk.ErrInternal.Wrapf("failed to set account number: %v", err)
+    return ctx, sdkerrors.Wrapf(sdkerrors.ErrInvalidRequest, "failed to set account number: %v", err)
}
app/lanes/mempool.go (1)

Line range hint 155-182: Consider extracting constants and error messages.

The validation logic is solid, but would benefit from better organization:

  • Define error messages as constants
  • Consider creating a separate type for lane limits configuration
+const (
+    ErrTxSizeExceeded   = "transaction size %d exceeds max lane size %d"
+    ErrTxGasLimitExceeded = "transaction gas limit %d exceeds max lane gas limit %d"
+    ErrNotFeeTx         = "transaction does not implement FeeTx interface"
+)

+type LaneLimits struct {
+    MaxTxSize    int64
+    MaxGasLimit  uint64
+}

 func (cm *Mempool[C]) AssertLaneLimits(ctx sdk.Context, tx sdk.Tx) error {
     maxBlockSize, maxGasLimit := proposals.GetBlockLimits(ctx)
-    maxLaneTxSize := cm.ratio.MulInt64(maxBlockSize).TruncateInt64()
-    maxLaneGasLimit := cm.ratio.MulInt(math.NewIntFromUint64(maxGasLimit)).TruncateInt().Uint64()
+    limits := LaneLimits{
+        MaxTxSize:    cm.ratio.MulInt64(maxBlockSize).TruncateInt64(),
+        MaxGasLimit:  cm.ratio.MulInt(math.NewIntFromUint64(maxGasLimit)).TruncateInt().Uint64(),
+    }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e2fbf0f and 1687893.

📒 Files selected for processing (5)
  • app/ante/accnum/account_number.go (1 hunks)
  • app/ante/ante.go (2 hunks)
  • app/ante/sigverify/sigverify.go (2 hunks)
  • app/lanes/mempool.go (3 hunks)
  • app/lanes/proposals.go (3 hunks)
🔇 Additional comments (4)
app/ante/ante.go (1)

90-90: 🛠️ Refactor suggestion

⚠️ Potential issue

Ensure NewSetUpContextDecorator is the first decorator

According to Cosmos SDK best practices, NewSetUpContextDecorator must be the first decorator in the AnteHandler chain. Currently, it's placed after accnum.NewAccountNumberDecorator. To maintain the correct execution order, NewSetUpContextDecorator should be the first decorator.

Apply this diff to reorder the decorators:

 func buildAnteDecorators(options HandlerOptions, freeLaneFeeChecker func(sdk.Context, sdk.Tx) (sdk.Coins, int64, error)) []sdk.AnteDecorator {
     return []sdk.AnteDecorator{
+        ante.NewSetUpContextDecorator(), // Must be the first decorator.
         accnum.NewAccountNumberDecorator(options.AccountKeeper),
-        ante.NewSetUpContextDecorator(), // Must be the first decorator.
         ante.NewExtensionOptionsDecorator(options.ExtensionOptionChecker),
         moveante.NewGasPricesDecorator(),
         // ... remaining decorators
     }
 }

Likely invalid or redundant comment.

app/ante/sigverify/sigverify.go (1)

90-98: Handle missing pubKey in simulation and ReCheckTx

In simulation or ReCheckTx modes, if pubKey is nil, signature verification is skipped. Ensure that this behavior is intended and that it doesn't lead to security issues by allowing transactions without public keys in certain modes.

app/lanes/mempool.go (2)

23-29: LGTM! Well-structured struct definition with clear documentation.

The Mempool struct is well-organized with appropriate field types and clear documentation for each component. The use of generics with the comparable constraint is a good choice for type safety.


143-152: 🛠️ Refactor suggestion

Reconsider single-signer assumption in getTxKey.

The current implementation only uses the first signer for indexing, which might not be sufficient for multi-signer transactions. This could lead to unexpected behavior when transactions have multiple signers with different sequences.

Let's verify if multi-signer transactions are common in the codebase:

Consider either:

  1. Creating composite keys that include all signers
  2. Documenting the limitation and rationale for using only the first signer
✅ Verification successful

Based on the search results, I can now provide a final response:

Using first signer for indexing is appropriate in this context.

The current implementation's approach of using the first signer for indexing is actually correct and consistent with the codebase's design:

  1. The mempool's role is transaction ordering and basic validation, not signature verification
  2. Full signature verification happens in the ante handler (app/ante/sigverify/), which properly validates all signers
  3. The mempool key is used for deduplication and ordering, where the first signer's sequence number is sufficient

The code shows a clear separation of concerns:

  • Mempool: Basic indexing and ordering using first signer
  • Ante handler: Complete signature verification for all signers
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for multi-signer transaction patterns
ast-grep --pattern 'GetSigners($$$)' | grep -B 2 -A 2 'Signers'

Length of output: 63


Script:

#!/bin/bash
# Let's try a different approach to find multi-signer related code
# First, let's look for GetSigners method implementations
rg -A 5 "func.*GetSigners" 

# Also search for any mentions of multiple signers in comments or code
rg -i "multi.*signer|multiple.*signer"

# Look for test cases that might involve multiple signers
rg "test.*signer" -A 5

Length of output: 65851


Script:

#!/bin/bash
# Let's search for any code or comments related to transaction signers
# Search for files containing GetSigners implementation
rg -p "func.*GetSigners" 

# Look for any test files or code related to transaction signing
rg -p "type.*Tx.*interface" -A 5

# Search for any comments or documentation about transaction signing
rg -i "transaction.*sign|tx.*sign" 

Length of output: 65634


Script:

#!/bin/bash
# Let's search for transaction signing related code in the app directory
rg -p "type.*Signer" app/
rg -p "interface.*Signer" app/
rg -p "struct.*Signer" app/

# Look for any documentation or comments about transaction signing
rg -i "transaction.*signer|tx.*signer" app/ -A 3

Length of output: 4176

Comment on lines +38 to 40
if err := validateHandlerOptions(options); err != nil {
return nil, err
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Validate all required dependencies in validateHandlerOptions

Currently, validateHandlerOptions checks for AccountKeeper, BankKeeper, and SignModeHandler. However, additional dependencies like ExtensionOptionChecker, FeegrantKeeper, IBCkeeper, AuctionKeeper, TxEncoder, MoveKeeper, MevLane, and FreeLane are used later in the code without prior validation. To prevent potential nil pointer dereferences, consider adding validation for these dependencies.

Comment on lines +83 to +84
return moveante.NewMempoolFeeChecker(options.MoveKeeper).CheckTxFeeWithMinGasPrices
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Check for nil MoveKeeper in getTxFeeChecker

In getTxFeeChecker, options.MoveKeeper is used without checking if it's nil. If MoveKeeper is nil, this will cause a nil pointer dereference at runtime. Ensure that MoveKeeper is validated in validateHandlerOptions.

Comment on lines 96 to 97
ante.NewConsumeGasForTxSizeDecorator(options.AccountKeeper),
ante.NewDeductFeeDecorator(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper, freeLaneFeeChecker),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Validate FeegrantKeeper before usage

In line 97, NewDeductFeeDecorator is called with options.FeegrantKeeper. If FeegrantKeeper is nil, this could result in a runtime error. Add a check in validateHandlerOptions to ensure FeegrantKeeper is not nil.

Comment on lines +174 to 176
return consumeMultisignatureVerificationGas(meter, sig.Data.(*signing.MultiSignatureData), pubkey, params, sig.Sequence)

default:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Ensure safe type assertion for multisignature data

In DefaultSigVerificationGasConsumer, when handling multisignature public keys, you assert that sig.Data is of type *signing.MultiSignatureData without checking. To prevent potential panics, ensure the type assertion is safe or handle the error appropriately.

Apply this diff to safely handle the type assertion:

 case multisig.PubKey:
-    return consumeMultisignatureVerificationGas(meter, sig.Data.(*signing.MultiSignatureData), pubkey, params, sig.Sequence)
+    multiSigData, ok := sig.Data.(*signing.MultiSignatureData)
+    if !ok {
+        return errorsmod.Wrap(sdkerrors.ErrInvalidType, "expected MultiSignatureData for multisig public key")
+    }
+    return consumeMultisignatureVerificationGas(meter, multiSigData, pubkey, params, sig.Sequence)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return consumeMultisignatureVerificationGas(meter, sig.Data.(*signing.MultiSignatureData), pubkey, params, sig.Sequence)
default:
multiSigData, ok := sig.Data.(*signing.MultiSignatureData)
if !ok {
return errorsmod.Wrap(sdkerrors.ErrInvalidType, "expected MultiSignatureData for multisig public key")
}
return consumeMultisignatureVerificationGas(meter, multiSigData, pubkey, params, sig.Sequence)
default:

app/ante/accnum/account_number.go Show resolved Hide resolved
Comment on lines +72 to +74
// Priority returns the transaction priority.
func (cm *Mempool[C]) Priority(ctx sdk.Context, tx sdk.Tx) any {
return 1
return 1 // Fixed priority for now; extend as needed.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Priority implementation needs improvement.

The current implementation returns a constant value of 1, which may not provide optimal transaction ordering. Consider implementing a more sophisticated priority mechanism based on factors like:

  • Transaction fees
  • Gas prices
  • Account sequence
  • Transaction type

Would you like help implementing a more comprehensive priority calculation?

Comment on lines +92 to 141
// Contains checks whether a transaction exists in the mempool.
func (cm *Mempool[C]) Contains(tx sdk.Tx) bool {
if key, err := cm.getTxKey(tx); err != nil {
key, err := cm.getTxKey(tx)
if err != nil {
return false
} else {
if _, ok := cm.txCache[key]; ok {
return true
} else {
return false
}
}
_, exists := cm.txCache[key]
return exists
}

// Insert inserts a transaction into the mempool.
// Insert adds a transaction to the mempool after validating lane limits.
func (cm *Mempool[C]) Insert(ctx context.Context, tx sdk.Tx) error {
if err := cm.AssertLaneLimits(sdk.UnwrapSDKContext(ctx), tx); err != nil {
sdkCtx := sdk.UnwrapSDKContext(ctx)

// Validate lane limits.
if err := cm.AssertLaneLimits(sdkCtx, tx); err != nil {
return err
}

// Insert into the underlying priority mempool.
if err := cm.index.Insert(ctx, tx); err != nil {
return fmt.Errorf("failed to insert tx into auction index: %w", err)
}

if key, err := cm.getTxKey(tx); err != nil {
// Cache the transaction.
key, err := cm.getTxKey(tx)
if err != nil {
return err
} else {
cm.txCache[key] = struct{}{}
}
cm.txCache[key] = struct{}{}

return nil
}

// Remove removes a transaction from the mempool.
// Remove deletes a transaction from the mempool and its cache.
func (cm *Mempool[C]) Remove(tx sdk.Tx) error {
// Remove from the priority mempool.
if err := cm.index.Remove(tx); err != nil && !errors.Is(err, sdkmempool.ErrTxNotFound) {
return fmt.Errorf("failed to remove transaction from the mempool: %w", err)
}

if key, err := cm.getTxKey(tx); err != nil {
// Remove from the cache.
key, err := cm.getTxKey(tx)
if err != nil {
return err
} else {
delete(cm.txCache, key)
}
delete(cm.txCache, key)

return nil
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Add mutex protection for thread-safe cache operations.

The txCache map is accessed from multiple methods (Contains, Insert, Remove) without synchronization. This could lead to race conditions in a concurrent environment.

 type Mempool[C comparable] struct {
     index     sdkmempool.Mempool
     extractor signer_extraction.Adapter
     txCache   map[txKey]struct{}
     ratio     math.LegacyDec
     txEncoder sdk.TxEncoder
+    mu        sync.RWMutex    // Protects txCache
 }

 func (cm *Mempool[C]) Contains(tx sdk.Tx) bool {
     key, err := cm.getTxKey(tx)
     if err != nil {
         return false
     }
+    cm.mu.RLock()
+    defer cm.mu.RUnlock()
     _, exists := cm.txCache[key]
     return exists
 }

Committable suggestion skipped: line range outside the PR's diff.

ante.NewValidateSigCountDecorator(options.AccountKeeper),
ante.NewSigGasConsumeDecorator(options.AccountKeeper, sigGasConsumer),
ante.NewSigGasConsumeDecorator(options.AccountKeeper, sigverify.DefaultSigVerificationGasConsumer),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change sigGasConsumer to retrieve from options first?

Copy link
Contributor

@beer-1 beer-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sigGasConsumer should fetch from options first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants